[codex] Structure preview automation errors - #3272
Conversation
Co-authored-by: codex <codex@users.noreply.github.com>
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
github-actions
Bot
added
size:L
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Disconnect masked as queue closed
- Used Deferred.poll when Queue.offer returns false to check if the deferred was already completed with PreviewAutomationClientDisconnectedError by disconnect(), falling back to PreviewAutomationRequestQueueClosedError only when the deferred is still pending.
Or push these changes by commenting:
@cursor push e0c5ec88bf
Preview (e0c5ec88bf)
diff --git a/apps/server/src/mcp/PreviewAutomationBroker.ts b/apps/server/src/mcp/PreviewAutomationBroker.ts
--- a/apps/server/src/mcp/PreviewAutomationBroker.ts
+++ b/apps/server/src/mcp/PreviewAutomationBroker.ts
@@ -375,7 +375,11 @@
timeoutMs,
});
if (!offered) {
- return yield* new PreviewAutomationRequestQueueClosedError(requestContext);
+ const completed = yield* Deferred.poll(deferred);
+ return yield* Option.match(completed, {
+ onNone: () => Effect.fail(new PreviewAutomationRequestQueueClosedError(requestContext)),
+ onSome: (effect) => effect as Effect.Effect<A, PreviewAutomationError>,
+ });
}
const result = yield* Deferred.await(deferred).pipe(Effect.timeoutOption(timeoutMs));
return yield* Option.match(result, {You can send follow-ups to the cloud agent here.
Reviewed by Cursor Bugbot for commit 472d593. Configure here.
ApprovabilityVerdict: Approved This PR refactors preview automation error classes to include structured diagnostic fields instead of just message strings. The changes improve error observability without altering when or why errors are thrown, and comprehensive tests verify the new structure. You can customize Macroscope's approvability policy. Learn more. |
Co-authored-by: codex <codex@users.noreply.github.com>
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
Co-authored-by: codex <codex@users.noreply.github.com>
Dismissing prior approval to re-evaluate 195af59
Co-authored-by: codex <codex@users.noreply.github.com>
1 participant
Summary\n- attach MCP scope, provider, client, request, tab, timeout, and bounded selector context to preview automation failures\n- distinguish host, disconnect, queue, remote-unavailable, and malformed-response failures instead of collapsing them into message bags\n- retain remote tags plus message lengths/detail kinds without copying arbitrary remote messages, details, or selectors into direct fields/messages; preserve the exact remote failure payload only as the cause chain
Verification
vp test apps/server/src/mcp/PreviewAutomationBroker.test.tsvp check(passes with 20 pre-existing warnings)vp run typecheckNote
Medium Risk
Broker and contract error shapes change in ways that break
PreviewAutomationUnavailableErrormatching; behavior is still localized to preview automation/MCP paths with added tests.Overview
Preview automation failures are now typed, request-scoped errors instead of free-form
messagebags.Contract error classes gain MCP scope (
operation,environmentId,threadId,providerSessionId,providerInstanceId) and, when a request was in flight, client/request context (clientId,requestId,tabId,timeoutMs). Remote failures add bounded diagnostics (remoteTag,remoteMessageLength, optionalremoteDetailKind) and keep the wire payload oncause, without copying arbitrary remote messages or selector strings into surfaced fields.PreviewAutomationInvalidSelectorErrordrops the raw selector in favor ofselectorKind/selectorLength.PreviewAutomationBrokerstores that context on each pending request, usesclassifyResponseErrorto map remote tags (including legacyPreviewAutomationUnavailableError) to the right type, and splits former “unavailable” cases intoPreviewAutomationHostNotConnectedError,PreviewAutomationClientDisconnectedError,PreviewAutomationRequestQueueClosedError,PreviewAutomationRemoteUnavailableError, andPreviewAutomationMalformedResponseError. Tests assert the new shapes and that secrets do not leak intomessage.Breaking for callers: code that only matched
PreviewAutomationUnavailableErrorfrom the broker must handle the new tags; the old class remains for other uses (e.g. MCP capability checks).Reviewed by Cursor Bugbot for commit 9112391. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Structure preview automation errors with typed context and diagnostics
PreviewAutomationBrokerwith structured per-request context (PreviewAutomationRequestErrorContext) carrying operation, environment, thread, provider, client, request, timeout, and selector diagnostics.PreviewAutomationHostNotConnectedError,PreviewAutomationClientDisconnectedError,PreviewAutomationRequestQueueClosedError,PreviewAutomationRemoteUnavailableError, andPreviewAutomationMalformedResponseError.NoFocusedOwnerError,TimeoutError,ExecutionError,InvalidSelectorError, etc.) are extended with structured request/scope fields and dynamic message getters; raw selector values are replaced with boundedselectorKind/selectorLengthdiagnostics.classifyResponseErrorreplacesmakeResponseError, mapping remote failures to concrete typed errors with remote tag, message length, and detail kind metadata.PreviewAutomationUnavailableErrorfrom remote is now mapped toPreviewAutomationRemoteUnavailableError; client disconnection during a pending request now throwsPreviewAutomationClientDisconnectedErrorinstead ofPreviewAutomationUnavailableError.Macroscope summarized 9112391.